feat(EmptyDetailsView): add component #65
Conversation
- using "type" with Omit instead of "interface" doesn't work for some reason with docs-framework
There was a problem hiding this comment.
It's hard for me to gage how inconvenient it'll be that the test-id is not exposed any more. Once they migrate to PF6, they will have to cross that bridge themself if they decide it matters, so for now, I think it's okay to not worry about that one item in the hidden component.
| /** Source path to an icon image. */ | ||
| iconImage?: string; | ||
| /** Alternative text for icon image if image can't load. */ | ||
| imageAlt?: string; |
There was a problem hiding this comment.
Do we need alt text for this image? This image is purely decorative and doesn't provide any additional meaning to the user that isn't already provided with the contents on the page.
| /** Button which initiates the creation. */ | ||
| createButton?: React.ReactNode; | ||
| /** Extra children to render inside EmptyStateFooter. */ | ||
| footerExtraChildren?: React.ReactNode; |
There was a problem hiding this comment.
The example provided in the preview shows an empty state with just a single primary action, but it would be good to have an example that shows a secondary action, too.
| title="Start by adding cluster storage" | ||
| description="Cluster storage saves your project’s data on a selected cluster. You can optionally connect cluster storage to a workbench." | ||
| iconImage={clusterImage} | ||
| imageSize="240px" |
There was a problem hiding this comment.
Can we change 240px to 320px
Also, this size is different than the size shown in /EmptyDetailsView/EmptyDetailsView.tsx below, which is using 320px.
We have new guidelines for empty states that recommend using 320px for illustrations. It would be ideal if our examples in the preview use the values we are recommending.
|
Overall this looks good. I did have a few requests regarding the preview examples, and one question about image alt text that would be good to get someone else's opinion on. |
Closes #26
direct link to new docs: https://ai-infra-ui-components-pr-65.surge.sh/ai-infra-ui-components/emptydetailsview
Issues:
EmptyStateHeader(it is now an internal component in Patternfly), thusdata-testid="empty-state-title"must be removed, might be worth opening an issue in pf-react?